Skip to content

fix: align components with the real Etherpad colibris skin#4

Open
JohnMcLear wants to merge 4 commits into
mainfrom
fix/colibris-fidelity
Open

fix: align components with the real Etherpad colibris skin#4
JohnMcLear wants to merge 4 commits into
mainfrom
fix/colibris-fidelity

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Why

The components diverged from Etherpad's actual colibris skin. The color tokens were correct (#485365 / #576273 / #64d29b / #d2d2d2 / #f2f3f4 match the real palette), but several components applied them wrongly, so the UI didn't look like Etherpad.

Fixes (each checked against src/static/skins/colibris)

  • EpButton — primary was inverted. Real .btn-primary is a green (--primary-color) background with white (--bg-color) text; the component had a dark background with green text. Fixed; hover/active darken via brightness.
  • EpButton — UA background leak. The base button set no background, so the browser's light ButtonFace showed through default/ghost/icon — invisible in light themes, glaringly wrong in dark. Reset to transparent (matches Etherpad's background: none).
  • EpChatMessage — was a bubble redesign. Etherpad chat is a plain line (#chattext p, padding 4px 10px): bold author, muted inline time, then text — no bubbles, own/other identical. Ported.
  • EpEditor — looked like a code box. Defaulted to monospace + #333. Etherpad's editor is proportional in --text-color (#485365). Defaults now follow the theme.
  • Error/danger red. EpInput/EpNotification/EpToast used #d9534f; aligned to Etherpad .btn-danger #d1242f.

Audit of the rest

I scanned every component for hardcoded colors that bypass the theme. Aside from the above, the remaining components correctly use var(--token). Components without a direct Etherpad equivalent (card, color wheel, user badge) are left as palette-consistent extensions.

Verification

  • pnpm typecheck clean, pnpm test --run82/82.
  • Rendered the full component set in light + dark (screenshots below) — no console errors. Primary buttons are green, default/ghost/menu/toolbar buttons no longer wash out in dark, chat is plain inline, editor is proportional.

(Also carries the pnpm dlxpnpm exec Playwright CI fix so this branch's browser tests pass independently.)

🤖 Generated with Claude Code

JohnMcLear and others added 2 commits June 2, 2026 18:45
The flagged components diverged from the real colibris skin:

- EpButton primary was inverted (dark bg + green text). Real colibris
  .btn-primary is a primary-coloured (green) background with bg-coloured
  (white) text. Fixed; hover/active now darken via brightness filter.
- EpChatMessage was a bubble-style redesign. Etherpad chat is a plain
  message line (#chattext p, padding 4px 10px): bold author, muted inline
  time, then the text — no bubbles, own/other styled the same. Ported.
- EpEditor defaulted to a monospace font and #333 text. Etherpad's editor
  is proportional in --text-color (#485365). Defaults now follow the theme
  (--main-font-family / --text-color / --bg-color).

Also carries the pnpm dlx→exec Playwright CI fix so this branch's browser
tests pass independently of the docs PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- EpButton: the base button never set a background, so the browser's
  light ButtonFace leaked through default/ghost/icon variants — fine in
  light themes, glaringly wrong in dark. Reset to transparent (matches
  Etherpad's `background: none` base).
- EpInput/EpNotification/EpToast: error/danger colour was #d9534f; use
  Etherpad colibris .btn-danger #d1242f for consistency.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Align components with Etherpad colibris skin and fix CI

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Align EpButton primary variant to Etherpad colibris (green background, white text)
• Reset button background to transparent to prevent browser default leak in dark themes
• Redesign EpChatMessage from bubbles to plain inline format matching Etherpad
• Update EpEditor to use proportional font and theme colors instead of monospace
• Standardize error/danger color across components to Etherpad's #d1242f
• Fix Playwright CI to use pinned version instead of latest
Diagram
flowchart LR
  A["Components<br/>diverged from<br/>Etherpad"] -->|"Fix primary<br/>button styling"| B["EpButton<br/>green bg +<br/>white text"]
  A -->|"Reset UA<br/>background"| C["EpButton<br/>transparent base"]
  A -->|"Redesign to<br/>plain format"| D["EpChatMessage<br/>inline layout"]
  A -->|"Use theme<br/>colors"| E["EpEditor<br/>proportional font"]
  A -->|"Align danger<br/>color"| F["EpInput/Notification/<br/>Toast #d1242f"]
  G["CI Issue<br/>pnpm dlx"] -->|"Use pinned<br/>version"| H["pnpm exec<br/>Playwright"]

Loading

Grey Divider

File Changes

1. src/EpButton.ts 🐞 Bug fix +13/-9

Fix primary button and reset UA background

• Reset button background to transparent to prevent browser ButtonFace leak in dark themes
• Invert primary variant: green background with white text (was dark bg with green text)
• Replace primary hover/active with brightness filter (0.94 and 0.88) instead of background changes
• Update transition to only animate filter and opacity

src/EpButton.ts


2. src/EpChatMessage.ts 🐞 Bug fix +12/-23

Redesign chat to plain inline format

• Remove bubble-style redesign; revert to plain inline message format matching Etherpad
• Simplify layout from flex header/body structure to inline bold author, muted time, and text
• Adjust padding from 6px to 4px 10px to match Etherpad colibris
• Remove border-radius and margin from own messages; keep background tint only

src/EpChatMessage.ts


3. src/EpEditor.ts 🐞 Bug fix +3/-3

Use theme colors and proportional font

• Change default font from monospace to theme's main-font-family (proportional)
• Update text color from hardcoded #333 to theme's text-color (#485365)
• Update background from hardcoded #fff to theme's bg-color
• Maintain fallback chain for customization via CSS variables

src/EpEditor.ts


View more (4)
4. src/EpInput.ts 🐞 Bug fix +2/-2

Align error color to Etherpad danger

• Change error border color from #d9534f to #d1242f (Etherpad colibris danger)
• Change error text color from #d9534f to #d1242f for consistency

src/EpInput.ts


5. src/EpNotification.ts 🐞 Bug fix +2/-2

Align error color to Etherpad danger

• Change error notification border-left color from #d9534f to #d1242f
• Change error icon color from #d9534f to #d1242f

src/EpNotification.ts


6. src/EpToast.ts 🐞 Bug fix +2/-2

Align error color to Etherpad danger

• Change error toast SVG fill color from #d9534f to #d1242f
• Change error toast border-left color from #d9534f to #d1242f

src/EpToast.ts


7. .github/workflows/ci.yml 🐞 Bug fix +4/-1

Fix Playwright CI to use pinned version

• Replace pnpm dlx playwright install with pnpm exec playwright install
• Add comment explaining that pnpm exec uses project-pinned Playwright version
• Prevents mismatched browser revisions when latest Playwright is fetched

.github/workflows/ci.yml


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Jun 2, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. Empty time adds gap 🐞 Bug ≡ Correctness
Description
EpChatMessage.render() always inserts a <span class="time"> even when time is empty, and
.time has horizontal margins, so timestamp-less messages still get extra spacing between author
and body. This is a behavioral regression vs the previous conditional render and will make some chat
lines look misaligned.
Code

src/EpChatMessage.ts[50]

Evidence
The template explicitly renders an empty .time span when this.time is falsy, and the .time
rule applies margins regardless of content, so an empty span still consumes horizontal space.

src/EpChatMessage.ts[28-32]
src/EpChatMessage.ts[48-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EpChatMessage` renders an empty `<span class="time"></span>` when `time` is falsy. Because the `.time` CSS rule adds horizontal margins, messages without a timestamp still get extra left/right spacing, producing an unintended visual gap.

### Issue Context
The previous implementation only rendered the time element when `time` was present. The new implementation keeps the element even when empty, which changes layout.

### Fix Focus Areas
- src/EpChatMessage.ts[48-51]
- src/EpChatMessage.ts[28-32]

### Suggested fix
Either:
1) Revert to conditional rendering (render nothing when `time` is empty), OR
2) Keep the empty span but add CSS such as `.time:empty { display: none; }` (or remove margins when empty) so it does not create spacing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@JohnMcLear JohnMcLear requested a review from SamTV12345 June 2, 2026 18:37
JohnMcLear and others added 2 commits June 2, 2026 19:52
From a full component-by-component audit against the real colibris skin:

- EpCheckbox: same inversion as the button — checked track used
  --text-color (dark) instead of --primary-color. Rebuilt as Etherpad's
  outlined toggle: bg-soft track with a text-soft border + thumb that turn
  primary-coloured (green) when checked (matches form.css :before/:after).
- EpColorPicker: default swatches were generic saturated colours; replaced
  with Etherpad's pastel author-colour palette (AuthorManager.getColorPalette).
- EpModal: backdrop was rgba(72,83,101,.3); use Etherpad's dialog backdrop
  rgba(0,0,0,.45) + blur(2px) (#settings-dialog::backdrop).
- EpEditor: default link colour #0366d6 → Etherpad's #2e96f3.

Audit found the remaining components (input, toast, notification, dropdown,
toolbar-select, chat, user badge) already track the theme correctly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
EpEditor mounted Etherpad's Ace engine inside a shadow root. Ace reads and
restores the caret via the document Selection API, which does not work
across a shadow boundary: getRangeAt() retargets the range to the shadow
host, so the caret always read as offset 0 and every keystroke inserted at
the document start — typing "hello" produced "olleh".

- EpEditor now renders into the light DOM (createRenderRoot returns this),
  so the engine uses the standard, well-tested document-selection path.
  Styles moved from `static styles` into an inline <style> (scoped under the
  ep-editor element since :host no longer applies).
- AceEditor.getSelection() also gains a getComposedRanges({shadowRoots})
  path as defensive support for any future shadow-DOM mounting, instead of
  bailing on the shadow-DOM rangeCount===0 case.

Verified: typing, Enter, and mid-line insertion all produce correct text
with no console errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant